-
Notifications
You must be signed in to change notification settings - Fork 337
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: gateway-api v0.5.1 #1445
Conversation
Makefile
Outdated
|
||
### uninstall-gateway-api: Uninstall Gateway API CRDs from the K8s cluster. | ||
.PHONY: uninstall-gateway-api | ||
uninstall-gateway-api: go-mod-download-gateway-api |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why we need go-mod-download-gateway-api?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but its uninstall-gateway-api
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it weird that I'm being asked to update it first when I want to delete it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This package does not exist when uninstalling is mainly avoided.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is obviously not necessary.
I can get the name of the CRD in the following two ways
kubectl api-resources --api-group=gateway.networking.k8s.io -o name
or
kubectl get crds | awk '/gateway.networking.k8s.io/ {print $1}'
Then I can get the output:
gatewayclasses.gateway.networking.k8s.io
httproutes.gateway.networking.k8s.io
referencepolicies.gateway.networking.k8s.io
tcproutes.gateway.networking.k8s.io
tlsroutes.gateway.networking.k8s.io
udproutes.gateway.networking.k8s.io
Just need a for
loop, you can delete these CRD resources directly.
This does not require any dependencies, even an offline environment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not very convenient. There is also a gateway-api webhook.
Makefile
Outdated
|
||
GATEWAY_API_PACKAGE ?= sigs.k8s.io/gateway-api | ||
GATEWAY_API_VERSION ?= v0.5.1 | ||
GATEWAY_API_CRDS_LOCAL_PATH = $(shell go env GOPATH)/pkg/mod/$(GATEWAY_API_PACKAGE)@$(GATEWAY_API_VERSION)/config/crd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we already keep a copy of this CRD configuration in the our repo, why not just use this copy to install?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dependent updates are convenient. The CRDs in the repo as an example, and maybe we can delete it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should keep that copy for anyone to use offline
Codecov Report
@@ Coverage Diff @@
## master #1445 +/- ##
==========================================
+ Coverage 41.25% 41.35% +0.10%
==========================================
Files 82 82
Lines 7279 7261 -18
==========================================
Hits 3003 3003
+ Misses 3929 3911 -18
Partials 347 347
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
c459e74
to
664b161
Compare
|
||
### install: Install Gateway API into the K8s cluster from go mod. | ||
.PHONY: install-gateway-api | ||
install-gateway-api: go-mod-download-gateway-api |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's necessary since we have a local copy.
Type of change:
What this PR does / why we need it:
Pre-submission checklist: